From 8842b02cfd658eaaf8be194fa366bd60d2e949c5 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Mon, 5 May 2014 16:27:43 -0700 Subject: [PATCH] Cleanup ProcessBuilder, work towards getting tests passing --- libs/hamcrest-rust | 2 +- src/cargo/core/resolver.rs | 8 +- src/cargo/util/process_builder.rs | 201 +++++++++++++++++++----------- tests/support.rs | 6 +- tests/test_cargo_compile.rs | 8 +- 5 files changed, 144 insertions(+), 81 deletions(-) diff --git a/libs/hamcrest-rust b/libs/hamcrest-rust index de700414a..138de49f2 160000 --- a/libs/hamcrest-rust +++ b/libs/hamcrest-rust @@ -1 +1 @@ -Subproject commit de700414aab1aaa4461618ce7a516cb24a8e6665 +Subproject commit 138de49f217604d22e019afd71c529a7d76643f0 diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs index 1f5386716..c4e22b102 100644 --- a/src/cargo/core/resolver.rs +++ b/src/cargo/core/resolver.rs @@ -44,9 +44,9 @@ mod test { }; use core::{ - MemRegistry, Dependency, - Package + Package, + PackageSet }; use super::{ @@ -71,8 +71,8 @@ mod test { Dependency::new(name) } - fn registry(pkgs: Vec) -> MemRegistry { - MemRegistry::new(&pkgs) + fn registry(pkgs: Vec) -> PackageSet { + PackageSet::new(&pkgs) } #[test] diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs index e51129066..3c1b808c3 100644 --- a/src/cargo/util/process_builder.rs +++ b/src/cargo/util/process_builder.rs @@ -2,88 +2,149 @@ use std::os; use std::path::Path; use std::io; use std::io::process::{Process,ProcessConfig,ProcessOutput,InheritFd}; +use collections::HashMap; use ToCargoError; use CargoResult; #[deriving(Clone,Eq)] pub struct ProcessBuilder { - program: ~str, - args: Vec<~str>, - path: Vec<~str>, - cwd: Path + program: ~str, + args: Vec<~str>, + path: Vec<~str>, + env: HashMap<~str, ~str>, + cwd: Path } // TODO: Upstream a Windows/Posix branch to Rust proper static PATH_SEP : &'static str = ":"; impl ProcessBuilder { - pub fn args(mut self, arguments: &[~str]) -> ProcessBuilder { - self.args = Vec::from_slice(arguments); - self - } - - pub fn extra_path(mut self, path: Path) -> ProcessBuilder { - // For now, just convert to a string, but we should do something better - self.path.push(format!("{}", path.display())); - self - } - - pub fn cwd(mut self, path: Path) -> ProcessBuilder { - self.cwd = path; - self - } - - // TODO: clean all this up - pub fn exec(&self) -> io::IoResult<()> { - let mut config = ProcessConfig::new(); - - config.program = self.program.as_slice(); - config.args = self.args.as_slice(); - config.cwd = Some(&self.cwd); - config.stdout = InheritFd(1); - config.stderr = InheritFd(2); - - let mut process = try!(Process::configure(config)); - let exit = process.wait(); - - if exit.success() { - Ok(()) - } - else { - Err(io::IoError { - kind: io::OtherIoError, - desc: "process did not exit successfully", - detail: None - }) - } - } - - pub fn exec_with_output(&self) -> CargoResult { - let mut config = ProcessConfig::new(); - - println!("cwd: {}", self.cwd.display()); - - config.program = self.program.as_slice(); - config.args = self.args.as_slice(); - config.cwd = Some(&self.cwd); - - let os_path = try!(os::getenv("PATH").to_cargo_error("Could not find the PATH environment variable".to_owned(), 1)); - let path = os_path + PATH_SEP + self.path.connect(PATH_SEP); - - let path = [("PATH".to_owned(), path)]; - config.env = Some(path.as_slice()); - - println!("{:?}", config); - - Process::configure(config).map(|mut ok| ok.wait_with_output()).to_cargo_error("Could not spawn process".to_owned(), 1) - } + pub fn args(mut self, arguments: &[~str]) -> ProcessBuilder { + self.args = Vec::from_slice(arguments); + self + } + + pub fn extra_path(mut self, path: Path) -> ProcessBuilder { + // For now, just convert to a string, but we should do something better + self.path.push(format!("{}", path.display())); + self + } + + pub fn cwd(mut self, path: Path) -> ProcessBuilder { + self.cwd = path; + self + } + + // TODO: should InheritFd be hardcoded? + pub fn exec(&self) -> io::IoResult<()> { + let mut config = try!(self.build_config()); + let env = self.build_env(); + + // Set where the output goes + config.env = Some(env.as_slice()); + config.stdout = InheritFd(1); + config.stderr = InheritFd(2); + + let mut process = try!(Process::configure(config)); + let exit = process.wait(); + + if exit.success() { + Ok(()) + } + else { + Err(io::IoError { + kind: io::OtherIoError, + desc: "process did not exit successfully", + detail: None + }) + } + } + + // TODO: Match exec() + pub fn exec_with_output(&self) -> CargoResult { + let mut config = ProcessConfig::new(); + + config.program = self.program.as_slice(); + config.args = self.args.as_slice(); + config.cwd = Some(&self.cwd); + + let os_path = try!(os::getenv("PATH").to_cargo_error("Could not find the PATH environment variable".to_owned(), 1)); + let path = os_path + PATH_SEP + self.path.connect(PATH_SEP); + + let path = [("PATH".to_owned(), path)]; + config.env = Some(path.as_slice()); + + Process::configure(config).map(|mut ok| ok.wait_with_output()).to_cargo_error("Could not spawn process".to_owned(), 1) + } + + fn build_config<'a>(&'a self) -> io::IoResult> { + let mut config = ProcessConfig::new(); + + config.program = self.program.as_slice(); + config.args = self.args.as_slice(); + config.cwd = Some(&self.cwd); + + Ok(config) + } + + fn build_env(&self) -> ~[(~str, ~str)] { + let mut ret = Vec::new(); + + for (key, val) in self.env.iter() { + // Skip path + if key.as_slice() != "PATH" { + ret.push((key.clone(), val.clone())); + } + } + + match self.build_path() { + Some(path) => ret.push(("PATH".to_owned(), path)), + _ => () + } + + ret.as_slice().to_owned() + } + + fn build_path(&self) -> Option<~str> { + let path = self.path.connect(PATH_SEP); + + match self.env.find_equiv(&("PATH")) { + Some(existing) => { + if self.path.is_empty() { + Some(existing.to_owned()) + } + else { + Some(existing.as_slice() + PATH_SEP + path) + } + } + None => { + if self.path.is_empty() { + None + } + else { + Some(path) + } + } + } + } } pub fn process(cmd: &str) -> ProcessBuilder { - ProcessBuilder { - program: cmd.to_owned(), - args: vec!(), - path: vec!(), - cwd: os::getcwd() - } + ProcessBuilder { + program: cmd.to_owned(), + args: vec!(), + path: vec!(), + cwd: os::getcwd(), + env: system_env() + } +} + +fn system_env() -> HashMap<~str, ~str> { + let mut ret = HashMap::new(); + + for &(ref key, ref val) in os::env().iter() { + ret.insert(key.clone(), val.clone()); + } + + ret } diff --git a/tests/support.rs b/tests/support.rs index 78f983a62..42e461dc3 100644 --- a/tests/support.rs +++ b/tests/support.rs @@ -187,7 +187,7 @@ impl Execs { None => ham::success(), Some(out) => { match str::from_utf8(actual.as_slice()) { - None => Err(~"stdout was not utf8 encoded"), + None => Err("stdout was not utf8 encoded".to_owned()), Some(actual) => { ham::expect(actual == out, format!("stdout was `{}`", actual)) } @@ -199,7 +199,7 @@ impl Execs { impl ham::SelfDescribing for Execs { fn describe(&self) -> ~str { - ~"execs" + "execs".to_owned() } } @@ -209,7 +209,7 @@ impl ham::Matcher for Execs { match res { Ok(out) => self.match_output(&out), - Err(_) => Err(~"could not exec process") + Err(_) => Err("could not exec process".to_owned()) } } } diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index c30e997e0..30c2e92ae 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -24,10 +24,12 @@ test!(cargo_compile_with_explicit_manifest_path { }"#) .build(); - p.cargo_process("cargo-compile") - .args([~"--manifest-path", ~"Cargo.toml"]) - .exec_with_output() + println!("~~~~~~~"); + p.cargo_process("cargo") + .args(["compile".to_owned(), "--manifest-path".to_owned(), "Cargo.toml".to_owned()]) + .exec() .unwrap(); + println!("~~~~~~~"); assert_that(&p.root().join("target/foo"), existing_file()); -- 2.30.2